Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Document how BoxFutures / BoxStreams are often made #2887

Merged
merged 3 commits into from
Oct 4, 2024

Conversation

alamb
Copy link
Contributor

@alamb alamb commented Oct 3, 2024

Rationale

Unless you are already familiar with the Rust futures ecosystem and FuturesExt::boxed it is not always obvious how to create a BoxFuture

Specifically, we have APIs for reading parquet files, which involve BoxFutures that when a user clicks on the return type,

Screenshot 2024-10-03 at 9 22 37 AM

The documentation doesn't lead them to FutureExt::boxed:

Screenshot 2024-10-03 at 9 21 43 AM

Proposal

While a better improvement for our users is a full featured example (added in apache/arrow-rs#6505), I think this to propose improvements in this crate too to help the broader community

If the maintainers like this pattern, I am happy to add it for other similar types in Futures and Streams but I won't bother if you are not interested in this type of change

@@ -9,6 +9,11 @@ pub use core::future::Future;

/// An owned dynamically typed [`Future`] for use in cases where you can't
/// statically type your result or need to add some indirection.
///
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This follows the wording and pattern in std, for example https://doc.rust-lang.org/std/iter/struct.Cloned.html

This struct is created by the cloned method on Iterator. See its documentation for more.

Here is how it looks rendered:
Screenshot 2024-10-03 at 9 34 52 AM

/// This type is often created by the [`boxed`] method on [`FutureExt`]. See its documentation for more.
///
/// [`boxed`]: https://docs.rs/futures/latest/futures/future/trait.FutureExt.html#method.boxed
/// [`FutureExt`]: https://docs.rs/futures/latest/futures/future/trait.FutureExt.html
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this links back to docs.rs because futures-core does not depened on futures-util, where FutureExt is defined

@alamb
Copy link
Contributor Author

alamb commented Oct 3, 2024

FYI @taiki-e

@taiki-e
Copy link
Member

taiki-e commented Oct 3, 2024

Thanks! This seems reasonable to me.

@taiki-e taiki-e added docs 0.3-backport: pending The maintainer accepted to backport this to the 0.3 branch, but backport has not been done yet. labels Oct 3, 2024
@taiki-e
Copy link
Member

taiki-e commented Oct 3, 2024

Could you add similar docs to other boxed types?

@alamb
Copy link
Contributor Author

alamb commented Oct 3, 2024

Could you add similar docs to other boxed types?

Yes, I will do so

@alamb alamb changed the title Document how BoxFutures are often made Document how BoxFutures / BoxStreams are often made Oct 3, 2024
@alamb
Copy link
Contributor Author

alamb commented Oct 3, 2024

Could you add similar docs to other boxed types?

Yes, I will do so

Done

@taiki-e taiki-e merged commit bbfc1ed into rust-lang:master Oct 4, 2024
24 checks passed
@alamb alamb deleted the alamb/box_future_docs branch October 4, 2024 10:58
@alamb
Copy link
Contributor Author

alamb commented Oct 4, 2024

Thank you again @taiki-e

@taiki-e taiki-e mentioned this pull request Oct 5, 2024
@taiki-e taiki-e added 0.3-backport: completed and removed 0.3-backport: pending The maintainer accepted to backport this to the 0.3 branch, but backport has not been done yet. labels Oct 5, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants